Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/add desktop entries action #119

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

oknozor
Copy link
Contributor

@oknozor oknozor commented May 27, 2022

This add the ability to use desktop entries actions.

Not sure but, this might break pop-shell front end. Maybe I should add another variant to the PluginResponse instead of using Context ?

Also I am wondering if we should implement activation for destkop entries directly in the launcher or still delegate to clients ? (see: #118).

@oknozor oknozor force-pushed the feat/add-desktop-entries-action branch from 1bcc819 to cd5b4dd Compare May 27, 2022 08:52
@jacobgkau jacobgkau requested review from a team May 27, 2022 14:42
@jacobgkau jacobgkau self-assigned this May 27, 2022
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure but, this might break pop-shell front end.

This is indeed causing some breakage in the right-click menu for desktop entries. The actions are not properly formatted (I'm seeing the new-window action identifier instead of the Open a New Window name key), and none of the menu options can be clicked (which means they can't be selected at all.)

Screenshot from 2022-05-27 10-45-55

@jacobgkau jacobgkau removed their assignment May 27, 2022
@oknozor
Copy link
Contributor Author

oknozor commented May 27, 2022

I think it would be better to have desktop entries ans actions activated by pop-launcher as mentioned here #118

I will try to tackle this soon, and make a PR in pop-shell in parallel.

@oknozor oknozor force-pushed the feat/add-desktop-entries-action branch from cd5b4dd to b33c9b3 Compare May 29, 2022 14:16
@oknozor
Copy link
Contributor Author

oknozor commented May 29, 2022

I updated the desktop entries plugin so it now take care of launching the entries, actions and gpu preferences.

Still need to figure out how to properly handle GPU selection for nvidia though. I don't have an nvidia machine so I won't be able to test this.

Should I implement something like this: https://gitlab.freedesktop.org/hadess/switcheroo-control/-/blob/master/src/switcheroo-control.c#L285 ?

Could you point me in the right direction @mmstick.

Also this will require to remove all the desktop entry related code in pop-shell. I'll send the PR soon.

@oknozor oknozor force-pushed the feat/add-desktop-entries-action branch from b33c9b3 to d0622a4 Compare May 30, 2022 06:45
@oknozor oknozor marked this pull request as draft May 31, 2022 13:21
@oknozor
Copy link
Contributor Author

oknozor commented May 31, 2022

@mmstick I have started to implement the Gpu switching logic to be able to launch desktop entries with the appropriate gpu (without relying on gnome stuff).

Also launching DE without the gnome helpers requires to implement a small part of the destkop entry spec. I have started to tackle this but it's not completely ready yet.

This is definitely no ready yet but the more I progress on this the more it feels like the desktop entry launching should reside in a separate crate.

I would totally understand if you don't want to change the current behavior of pop-launcher but I'd like to have your take on this, I don't want to spend more time on this if you are not considering to merge whenever it will be ready.
In my opinion removing some gtk/gnome dependencies is worth the effort in the long run.

Pros:

  • Non pop-launcher client such as onagre will profit from this immediately by removing the DE launching related code entirely.
  • Future client won't have to deal with this and can focus exclusively on writing UIs
    Cons:
  • Need to maintain the desktop entry launching code
  • Need to maintain the GPU switching related code

@mmstick
Copy link
Member

mmstick commented May 31, 2022

Switcheroo is a great guide for launching with hybrid graphics support.

It's fine to have a separate crate for launching desktop entries. There's multiple locations where it'd make sense to have that, outside of the pop-launcher.

@oknozor
Copy link
Contributor Author

oknozor commented May 31, 2022

I did took some inspiration from switcheroo, the main difference is that my implementation does not exposes gpu info on dbus, I guess it's fine ?

https://github.com/oknozor/launcher/blob/feat/add-desktop-entries-action/plugins/src/desktop_entries/graphics.rs

I was able to get my hand on a hybrid graphics nvidia graphics laptop so I should be able to test it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants